fix(h2): settle a request whose stream is cancelled - #5607
Merged
Merged
Conversation
RST_STREAM(CANCEL) received before the response is the one reset code Node reports as a bare 'close' on the client stream: RST NO_ERROR -> 'end', 'close' RST PROTOCOL_ERROR -> 'error', 'close' RST INTERNAL_ERROR -> 'error', 'close' RST REFUSED_STREAM -> 'error', 'close' RST CANCEL -> 'close' <- nothing to act on Destroying the stream also unenrolls its timeout, so no 'timeout' follows either. Since a request was only ever completed from 'end', 'error' or 'timeout', a cancelled stream left it in the running window forever and its caller never heard back -- no response, no error, no timeout, whatever headersTimeout and bodyTimeout were set to. Proxies send this upstream whenever their own downstream client goes away, so it is reachable against ordinary infrastructure. Settle the request from 'close' when nothing else has, mirroring what the 'error' handler already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A49JamgF2TkZHu5h58ChUM Signed-off-by: Matteo Collina <hello@matteocollina.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v7.x #5607 +/- ##
==========================================
- Coverage 93.17% 93.16% -0.01%
==========================================
Files 112 112
Lines 36751 36759 +8
==========================================
+ Hits 34241 34247 +6
- Misses 2510 2512 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
metcoder95
approved these changes
Jul 30, 2026
Merged
This was referenced Sep 23, 2026
meta-codesync Bot
pushed a commit
to facebook/memlab
that referenced
this pull request
Sep 29, 2026
Summary: Bumps [[ https://github.com/nodejs/undici | undici ]] from 7.29.0 to 7.30.0 in the memlab `website/` workspace. `yarn.lock` change only; it also picks up the 7.29.1 security release. **v7.30.0** - fix: selectively re-enable SIMD for ppc64 ([[ nodejs/undici#5794 | #5794 ]]) - Backport upgrade diagnostics lifecycle fixes ([[ nodejs/undici#5783 | #5783 ]]) - fix: honor backpressure in the decompression interceptor ([[ nodejs/undici#5837 | #5837 ]]) - fix: close rejected HTTP/2 WebSocket streams ([[ nodejs/undici#5876 | #5876 ]]) - test(fetch): make pull-dont-push exceed any socket buffer ([[ nodejs/undici#5889 | #5889 ]]) **v7.29.1 — security fixes** High severity: - [[ GHSA-w293-vg96-wgc3 | GHSA-w293-vg96-wgc3 ]]: `BalancedPool` could drop function-valued connection options, including custom TLS certificate validation callbacks, when cloning its configuration - [[ GHSA-rfgv-xxqx-mfg5 | GHSA-rfgv-xxqx-mfg5 ]]: a WebSocket server selecting a subprotocol when none was requested caused an uncaught `TypeError` that could terminate the process Medium severity: - [[ GHSA-3wwx-pv8p-q78v | GHSA-3wwx-pv8p-q78v ]]: a permessage-deflate payload over the decompression limit could emit an unhandled zlib error and terminate the process - [[ GHSA-rx4f-c7p8-82vq | GHSA-rx4f-c7p8-82vq ]]: an unclean `WebSocketStream` close with a locked writable stream could create an unobserved rejected promise - [[ GHSA-2jfj-6hjv-fm6j | GHSA-2jfj-6hjv-fm6j ]]: shared caches could store and replay responses containing `Set-Cookie`, disclosing one user's cookies to another caller - [[ GHSA-3xpg-4rpp-hhhm | GHSA-3xpg-4rpp-hhhm ]]: the decompression interceptor did not bound decoded output; every stage is now limited to 64 MiB by default, configurable via `maxSize` - [[ GHSA-pmjh-fq2x-6v4x | GHSA-pmjh-fq2x-6v4x ]]: a terminal retry failure after response headers were exposed could orphan the response body and hang consumers Low severity: - [[ GHSA-8436-99hf-9mmv | GHSA-8436-99hf-9mmv ]]: cache interceptors could store and replay responses to unsafe methods such as `POST` or `DELETE` - [[ GHSA-2gqq-gqf2-x968 | GHSA-2gqq-gqf2-x968 ]]: the dump interceptor could treat an oversized chunked response without `Content-Length` as successfully truncated - [[ GHSA-r53p-7pc4-xj5r | GHSA-r53p-7pc4-xj5r ]]: the retry interceptor could concatenate a resumed response with inconsistent framing, enabling response splitting or corruption **v7.29.1 — other changes** - fix(h2): honour `headersTimeout` ([[ nodejs/undici#5604 | #5604 ]]) - fix(h2): keep the connection ref'd while requests are outstanding ([[ nodejs/undici#5605 | #5605 ]]) - fix(h2): retire the request that completed, not the head of the queue ([[ nodejs/undici#5618 | #5618 ]]) - fix(h2): settle a request whose stream is cancelled ([[ nodejs/undici#5607 | #5607 ]]) - fix(h2): handle GOAWAY for CONNECT streams ([[ nodejs/undici#5640 | #5640 ]]) - perf: reduce `EventSourceStream` parser allocations ([[ nodejs/undici#5646 | #5646 ]]) - perf(h1): drop the idle-socket timer floor with a ref'd `setImmediate` ([[ nodejs/undici#5769 | #5769 ]]) - CI only: drop Node.js 26 from the shared-builtin build ([[ nodejs/undici#5592 | #5592 ]]); raise the Windows workflow timeout ([[ nodejs/undici#5621 | #5621 ]]) Full changelog: [[ nodejs/undici@v7.29.0...v7.30.0 | v7.29.0...v7.30.0 ]] Opened by Dependabot. Comment `dependabot rebase` on the GitHub PR to resolve conflicts; do not hand-edit the PR branch. Pull Request resolved: #155 Differential Revision: D122269178 Pulled By: JacksonGL fbshipit-source-id: 56fb568c08248253c483c6b44b1175af4a6b6d0c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A request over HTTP/2 was only ever completed from the stream's
'end','error'or'timeout'. There is one case that produces none of them, and it strands the request forever.What Node reports per reset code
Client sends a GET, server RSTs the stream before responding (pure
node:http2, no undici):CANCELis the code Node itself uses when a stream is destroyed locally, so an incomingRST_STREAM(CANCEL)is treated as a plain teardown rather than an error. Destroying the stream also unenrolls its timeout, so no'timeout'follows either —stream.setTimeout()is silently cancelled.The result, with
headersTimeoutandbodyTimeoutboth set to 500ms:No response, no error, no timeout, regardless of the configured timeouts — the caller's promise simply never settles and the request stays in the running window for the life of the client.
This is reachable against ordinary infrastructure: a proxy cancels its upstream stream whenever its own downstream client goes away.
Change
Settle the request from
'close'when nothing else has, mirroring what the'error'handler already does. The guard is!request.aborted && !request.completed, so normal completions and existing error paths are untouched.Tests
test/http2-stream-cancel.jscovers both directions: cancelled before the response (must reject) and cancelled after the headers arrive (must still complete normally). Note the first test hangs rather than failing on the current branch — that is the bug.test/+(http2|h2)*.js: 53 pass. Full unit suite: 1306 pass, 0 fail.A seeded chaos harness over the same code goes from requests that never settle to
stuck=0across every seed tried (7, 1, 42, 99).Still outstanding
That harness still reports queue drift on this branch —
runningstays above zero with nothing in flight — because the abort path settles the caller without advancingkRunningIdx, and the queue is head-of-line only (client[kQueue][client[kRunningIdx]++] = null) while h2 completes out of order. That predates this change and is not affected by it, but it means a long-lived client can still accumulate phantom running slots. Happy to look at it separately.🤖 Generated with Claude Code
https://claude.ai/code/session_01A49JamgF2TkZHu5h58ChUM